Add poison state to sandbox #931

ludfjig · 2025-10-02T19:12:55Z

This PR adds a poison state to sandbox in order to prevent further operations when the sandbox is left in an inconsistent state that could compromise memory safety, data integrity, or security. The sandbox becomes poisoned when guest functions abort/panic or when host-initiated execution cancellation occurs, leaving behind leaked heap allocations, corrupted data structures, or unreleased resources. For example, interrupting execution while guest is allocating can leave the global allocation lock in an inconsistent state, making future allocations impossible in subsequent runs due to infinite locking/spinning.

Poisoned sandboxes will reject all further operations (guest calls, snapshots, memory mapping) until the inconsistent state is resolved through either restoring to a snapshot or manually (unsafely) clearing the poison state.

Closes #848

Round 2:

I've changed the docs and implementation slightly. is_poison_error has been made private. Removed unsafe way of clearing poision. Users of hyperlight are expected on-error to check if the sandbox is poisoned, and deal with it accordingly, for example.

let result = sandbox.call::<()>("guest_panic", ());
if result.is_err() {
    if sandbox.poisoned() {
        // Restore from snapshot to clear poison
        sandbox.restore(&snapshot)?;
        assert!(!sandbox.poisoned());
        
        // Sandbox is now usable again
        sandbox.call::<String>("Echo", "hello".to_string())?;
    }
}

Copilot

Pull Request Overview

This PR introduces sandbox poisoning functionality to prevent further operations when a sandbox is in an inconsistent state that could compromise memory safety. The sandbox becomes poisoned when guest functions abort/panic or when host-initiated execution cancellation occurs.

Key changes:

Added poisoned state tracking with safety checks across all sandbox operations
Implemented automatic poison detection for specific error types (GuestAborted, ExecutionCanceledByHost)
Added recovery mechanisms through snapshot restoration or manual poison clearing

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/hyperlight_host/src/sandbox/initialized_multi_use.rs	Core implementation of poison state tracking, safety checks, and recovery mechanisms
src/hyperlight_host/src/error.rs	Added PoisonedSandbox error variant with detailed documentation
src/hyperlight_host/tests/integration_test.rs	Updated interrupt tests to clear poison state for continued execution
src/hyperlight_host/src/sandbox/snapshot.rs	Added Debug trait to Snapshot struct
src/hyperlight_host/src/mem/shared_mem_snapshot.rs	Added Debug trait to SharedMemorySnapshot struct

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

dblnz

Great stuff!
Changes LGTM. I left some clarifying comments, but overall good to go.

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

jsturtevant

Overall LGTM, a few nits but happy to see what other say

jprendes

This looks good to me!
Thanks @ludfjig !

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

ludfjig · 2025-10-27T19:32:56Z

I am all for giving the user control, however I don't see a scenario where a user could resolve this without calling restore or creating a new sandbox, what did you have in mind?

Not exactly sure. Maybe some usecase for completely state-less guests? Not sure tbh.

simongdavies · 2025-10-27T20:34:33Z

I am all for giving the user control, however I don't see a scenario where a user could resolve this without calling restore or creating a new sandbox, what did you have in mind?

Not exactly sure. Maybe some usecase for completely state-less guests? Not sure tbh.

No idea why this doesn't show up as a review comment, anyway my instinct is not to add this now and then if someone wants it add it later, seems like its safer that way

Signed-off-by: Ludvig Liljenberg <[email protected]>

old

Signed-off-by: Ludvig Liljenberg <[email protected]>

ludfjig force-pushed the poison branch from 531b913 to 127a080 Compare October 2, 2025 19:15

ludfjig added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Oct 2, 2025

ludfjig marked this pull request as ready for review October 2, 2025 19:15

ludfjig requested review from danbugs, dblnz, devigned, jprendes, marosset, simongdavies and syntactically as code owners October 2, 2025 19:15

ludfjig requested a review from Copilot October 2, 2025 20:07

Copilot AI reviewed Oct 2, 2025

View reviewed changes

src/hyperlight_host/src/sandbox/initialized_multi_use.rs Outdated Show resolved Hide resolved

src/hyperlight_host/src/sandbox/initialized_multi_use.rs Outdated Show resolved Hide resolved

dblnz previously approved these changes Oct 23, 2025

View reviewed changes

src/hyperlight_host/src/sandbox/initialized_multi_use.rs Outdated Show resolved Hide resolved

src/hyperlight_host/src/sandbox/initialized_multi_use.rs Outdated Show resolved Hide resolved

src/hyperlight_host/src/sandbox/initialized_multi_use.rs Outdated Show resolved Hide resolved

ludfjig dismissed dblnz’s stale review via 4aa73b1 October 23, 2025 21:22

ludfjig force-pushed the poison branch 3 times, most recently from 61713f3 to 74a9260 Compare October 23, 2025 22:32